Skip to content

Sort profile refresh tokens by 'last used at' date (#4484)#7199

Merged
zsarnett merged 1 commit intohome-assistant:devfrom
aav7fl:dev
Oct 11, 2020
Merged

Sort profile refresh tokens by 'last used at' date (#4484)#7199
zsarnett merged 1 commit intohome-assistant:devfrom
aav7fl:dev

Conversation

@aav7fl
Copy link
Copy Markdown
Contributor

@aav7fl aav7fl commented Oct 2, 2020

Proposed change

The user profile page lists are tokens created by/used by said user. It's a bit of a mess when trying to find tokens that were recently used (and clearing out older tokens). This change sorts in ascending order the tokens by their last used at date.

Previously there was no sorting mechanism (although it was sorted by chance with created dated it appears).

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

No configuration change needed.

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@Misiu
Copy link
Copy Markdown
Contributor

Misiu commented Oct 2, 2020

Take a look at #7195.
You should probably cast last_used_at as Date.
This was suggested by @bramkragten in my PR.

@aav7fl
Copy link
Copy Markdown
Contributor Author

aav7fl commented Oct 2, 2020

last_used_at

Thanks! Great catch 🙂

@aav7fl
Copy link
Copy Markdown
Contributor Author

aav7fl commented Oct 2, 2020

@bramkragten Should be good to go now. Looks like I have linting to fix.

  • Moved the function out of the render
  • Converted it to an arrow function
  • Added strong typing so the comparator doesn't get used accidentally by something else
  • Used a new Date() for the object suggested by @Misiu and yourself

@aav7fl aav7fl requested a review from bramkragten October 2, 2020 13:26
${this.hass.localize("ui.panel.profile.refresh_tokens.description")}
${refreshTokens?.length
? refreshTokens!.map(
? refreshTokens!.sort(compareTokenLastUsedAt).map(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should do the sorting in the memoizeOne, to prevent doing it on every render, and only do it when the tokens are changed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. So change:

refreshTokens?.filter((token) => token.type === "normal").reverse()

to

refreshTokens?.filter((token) => token.type === "normal").sort(compareTokenLastUsedAt)

?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private _sortedTokens = memoizeOne((tokens) => tokens.sort(compareTokenLastUsedAt).map())

Something like this. Break it into a private function

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a memoize function already :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aav7fl yes, thats perfect!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bramkragten Change pushed. Thanks for walking through with me on this.

@zsarnett zsarnett merged commit 934c227 into home-assistant:dev Oct 11, 2020
@bramkragten bramkragten mentioned this pull request Oct 21, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sort the Refresh Tokens card on profile page by Last used rather than creation date

5 participants